Skip to content

[6.x] PHPStan#14925

Merged
jasonvarga merged 28 commits into
6.xfrom
phpstan
Jul 3, 2026
Merged

[6.x] PHPStan#14925
jasonvarga merged 28 commits into
6.xfrom
phpstan

Conversation

@jasonvarga

@jasonvarga jasonvarga commented Jul 2, 2026

Copy link
Copy Markdown
Member

This adds PHPStan as a GitHub action workflow.

It uses level 0 for now to get things going. Targeting level 1 is a bigger undertaking and more realistic for v7.

In addition to just adding PHPStan, this also fixes a bunch of simple non-breaking errors. It brings it down from 270 to 70. (With those 70 in the baseline so we're at zero.)

jasonvarga and others added 28 commits July 2, 2026 11:37
Marks classes whose constructor is compatible across their subclass
hierarchy, so PHPStan can verify new static() calls are safe.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Facade @method docblocks lacking a return type were resolved as
instance methods instead of static ones.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
spatie/laravel-ignition is never a real dependency of this package —
it's an optional integration only present when the consuming Laravel
app installs it, guarded by isIgnitionInstalled()/class_exists()
checks in RuntimeParser.php. It can't be installed as a dev
dependency either: this repo's Laravel 13/Symfony 8 requirements
conflict with even the latest stable ignition release.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Statamic\Console\Composer\Scripts::preUpdateCmd() type-hints
Composer\Script\Event purely for IDE/static-analysis benefit — the
class is always available at runtime when Composer invokes the
script, but composer/composer was never a real dependency of this
package and isn't worth adding just for this type hint.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tion

barryvdh/laravel-debugbar is never a real dependency of this package —
Statamic's View/Debugbar/* classes only activate when the consuming
app has it installed, guarded by class_exists()/isEnabled() checks.

Most of the resulting errors are simple ignoreErrors entries, but
PHPStan hard-refuses to ignore "extends/implements unknown class"
errors (PerformanceCollector.php, VariableCollector.php) via
ignoreErrors — it requires the symbol to actually resolve. Since those
files also contain real logic worth analyzing, excludePaths would
have thrown out too much; instead .phpstan/stubs/debugbar.php provides
minimal empty-shell declarations (DataCollector, ConfigCollector,
AssetProvider, Renderable, Resettable) via scanFiles, matching the
real php-debugbar API surface just enough for PHPStan to resolve the
class hierarchy.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Statamic\Auth\UserService has never existed in this repo's git
history (git log --all --follow -- "*UserService.php" returns
nothing) — this line has been broken since the file was carried
over from the pre-monorepo codebase in commit a817e26
(2017-11-21).

extract() isn't part of the parent DataCollection contract, and has
zero call sites anywhere in src/ or tests/, so it was silently dead
(and PHP-fatal if ever called) rather than causing a visible bug.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Statamic\View\Store was deleted in commit ada5c2e ("SSG fixes
(#3562)", 2021-04-23), but the container binding for it in
ViewServiceProvider::register() was never cleaned up afterward.

No other reference to Statamic\View\Store exists in src/ or tests/
(the many other Store::class matches in the codebase are unrelated
classes — Stache\Stores\*, Illuminate\Contracts\Cache\Store, etc.).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Both Statamic\Events\Stache\RepositoryItemInserted and
RepositoryItemRemoved, which TermTracker::subscribe()/insert()/
remove() depend on, were deleted in commit 7ceee82 ("Remove
outdated events", 2020-07-09).

TermTracker was never wired back up after that: EventServiceProvider
has it commented out (// \Statamic\Taxonomies\TermTracker::class, //
TODO), so subscribe() never runs and insert()/remove() are never
invoked. No tests reference it. Its responsibility (tracking term
associations) is already covered by the still-active
src/Listeners/UpdateTermReferences.php.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Statamic\Routing\Route was deleted wholesale in commit 1541884
("Rip out routing", 2020-01-27). instanceof UndefinedClass silently
evaluates to false in PHP rather than throwing, so this branch was
permanently unreachable rather than crashing anything.

Confirmed cascadeContent() can never produce a Route today — its only
producers (Http/Responses/DataResponse.php, FrontendController::
getLoadedRouteItem() via Data::find()/findByUri(), the password
protection controller) all resolve to Data/Entry-style objects, never
a Route instance.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
AssetContainer::save()/delete() and the Permission facade's boot()
docblock were typed as void but the underlying methods actually
return a value ($this|false, bool, and \Statamic\Auth\Permissions
respectively).
These call genuinely void methods as their final statement (or, for
failedValidation(), a method that unconditionally throws), so the
return keyword served no purpose and PHPStan correctly flagged the
result as unused.
SiteClear, CommitCommand, and InstallSsg each had a guard clause of
the form 'if (...) { return $this->info(...); }' followed by more
code. Since info()/error() are void, PHPStan flagged the return
value as unused, but simply stripping return would have let
execution fall through into the code the guard is meant to skip.
Split each into a void call followed by a bare return to preserve
the early exit.
Fieldtype::preloadable() and AbstractSourceEmitter's stack-cleanup
logic both reference static::$property as an optional override
point, but the base classes never declared the properties
themselves - only some subclasses did (e.g. Relationship::$preloadable,
Blade/AntlersSourceEmitter::$variableStack).

Declared $preloadable untyped to match Relationship's untyped
override. Declared $variableStack/$currentIterationVar with the same
types (array/?string) as the Blade and Antlers subclasses - PHP
requires a typed property override to match its parent's type
exactly, so adding them untyped would have been a fatal error at
runtime for those two subclasses.
- Icon facade's sets() docblock claimed a required $name parameter,
  but the real IconManager::sets() takes none and returns all
  registered sets; all call sites already call it with no args.
- TreeBranchType references static::NAME as an override point but
  never declares it itself, only its concrete subclasses do. Same
  pattern as the earlier Fieldtype::$preloadable fix.
- NullIndex passed $this to NullSearchables's constructor, but
  NullSearchables has no constructor - the argument was silently
  discarded, not a real bug. Dropped the dead argument.
Same situation as the existing Spatie\LaravelIgnition\Exceptions
ignore entries: spatie/laravel-ignition is an optional runtime
integration this package never requires as a real dependency, and
ddd() is one of its global helpers (see
https://github.com/spatie/laravel-ignition/blob/main/src/helpers.php).
composer.json requires laravel/framework ^12.40.0 || ^13.0, and
Dispatcher::dispatchSync() has existed since long before that floor.
The method_exists() check (and its dispatchNow fallback, added in
2021 for Laravel versions this package no longer supports) always
resolves to dispatchSync, so it can be simplified to a plain
ternary. Also removes the now-unused Dispatcher import.
Carbon::macro('asVueComponent', ...) calls self::this() inside a
static closure, which Carbon\Traits\Mixin rebinds into scope for
macros at runtime (Mixin::this() is a real, protected static
method). PHPStan can't follow that scope rebinding and sees it as an
undefined static method on AppServiceProvider - confirmed via
empirical repro, including testing Carbon's own official
vendor/nesbot/carbon/extension.neon PHPStan extension, which does
not resolve this either.
- Removed the getOrPut() macro: Laravel's Collection has had a native
  getOrPut() with identical behavior for years, so the macro is
  permanently unreachable (native methods intercept before macro
  dispatch).
- Removed the pipe() macro: Laravel's Collection has had a native
  pipe() with an identical implementation since 5.2, long before this
  package's floor.
- keyByWithKey() is kept untouched, since it's a public macro
  third-party addons could still be calling even though Statamic's
  own code doesn't use it anymore. Its calls into Collection's
  protected valueRetriever()/$items are now suppressed via a scoped
  ignoreErrors entry instead of being rewritten.
- Everywhere else in the file, $this->items (protected) is replaced
  with $this->all() (public, returns the same value).
…ons macro

Illuminate\View\View::getEngine() is a public method that returns the
same protected $engine property this macro was accessing directly.
The 19-case switch had no default, so an unmatched $repository would
silently return null from a : bool-typed method - a TypeError risk
if allRepositories()'s keys and this switch's cases ever drift out
of sync. All 3 call sites currently only pass validated keys from
allRepositories(), so this is unreachable today, but throwing (rather
than returning false) means a future forgotten case fails loudly
instead of being misread as "not migrated yet".
mimeType(), lastModified(), fileSize(), listContents(), and
visibility() were empty stubs that always crashed with a TypeError
(no code path satisfies their strict return types). They had real
HEAD-request-based implementations before the Flysystem v1->v3
migration (#5551), but were converted to stubs during that migration
and never ported forward - the underlying head() request
infrastructure this class still has today was left unused for these
methods.

Confirmed unreachable in Statamic's current usage (Glide skips
validation for remote URLs; nothing in Flysystem's internals calls
these for this adapter), so there's no working behavior to restore.
Instead, throw Flysystem's own UnableToRetrieveMetadata/
UnableToListContents exceptions - the same contract other Flysystem
adapters use to signal a genuinely unsupported operation, rather than
crashing with a raw TypeError.
…crues

The remaining 70 errors (return.missing, method.notFound,
property.notFound, argument.unknown, etc.) are the abstract-pattern
and BC-sensitive cases deliberately left unfixed throughout this
branch - real, but requiring either a public API change or deeper
design decisions outside today's scope.

Baselining lets vendor/bin/phpstan analyse pass cleanly so the
GitHub Actions check is actually enforceable going forward: new
code introducing new errors will fail CI, while this known,
already-triaged debt stays visible in .phpstan/baseline.neon rather
than silently swallowed.

Placed inside .phpstan/ instead of the repo root to avoid clutter -
note PHPStan resolves a baseline's relative paths against its own
directory, so it was generated directly at this location rather than
moved after the fact.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@jasonvarga jasonvarga marked this pull request as ready for review July 3, 2026 02:32
@jasonvarga jasonvarga requested a review from a team as a code owner July 3, 2026 02:32
@jasonvarga jasonvarga merged commit 26bca1f into 6.x Jul 3, 2026
24 checks passed
@jasonvarga jasonvarga deleted the phpstan branch July 3, 2026 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants